Closed
Bug 696233
Opened 13 years ago
Closed 13 years ago
Consider having four separate hashtables with smaller entries in the nth-index cache
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file)
7.47 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
For querySelector this should give better cache locality (and is a slight performance improvement). For normal selector matching it might too, esp. in the common case when only one of the four types of selectors applies to a given node.
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #569646 -
Flags: review?(dbaron)
Assignee | ||
Updated•13 years ago
|
Attachment #569646 -
Flags: review?(dbaron) → review?(Olli.Pettay)
Comment 2•13 years ago
|
||
Comment on attachment 569646 [details] [diff] [review] Switch the nth-index cache to having a single hashtable per selector type (from end vs from start and of-type vs not-of-type). Gives somewhat better cache locality on some workloads and makes entry addition much cheaper because we no longer need to initi > nsNthIndexCache::IndexDeterminedFromPreviousSibling(nsIContent* aSibling, > Element* aChild, > bool aIsOfType, > bool aIsFromEnd, >+ const Cache &aCache, Cache& aCache would be consistent with other parameters. >+ // This node's index for this cache. >+ // If -2, needs to be computed. >+ // If -1, needs to be computed but known not to be 1. >+ // If 0, the node is not at any index in its parent. >+ typedef PRInt32 CacheEntry; huh, this code is...interesting. >+ /** >+ * Returns true if aResult has been set to the correct value for aChild and >+ * no more work needs to be done. Returns false otherwise. >+ */ Don't remove the comments you added in the other bug.
Attachment #569646 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 3•13 years ago
|
||
> Cache& aCache would be consistent with other parameters. Yeah, ok. Fixed. At some point I should just switch them all... > huh, this code is...interesting. dbaron gets the blame for that bit. :) Turns out, the "not 1" case is more common (e.g. used in our UA stylesheets). > Don't remove the comments you added in the other bug. Indeed. The patch here predates those comments being added; the actual patch to push has them. Thank you for the quick review! Pushed: http://hg.mozilla.org/integration/mozilla-inbound/rev/28afade47ac2
Flags: in-testsuite-
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
Whiteboard: [need review]
Target Milestone: --- → mozilla10
Comment 4•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/28afade47ac2
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•